Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: 카테고리별 케이크 이미지 조회 API 구현 #33

Merged
merged 7 commits into from
May 23, 2024

Conversation

lcomment
Copy link
Collaborator

Issue Number

#22

Description

카테고리 별 케이크 이미지 조회 API를 구현하였습니다. 통합테스트도 작성해보았는데, Template과 통합테스트 모두 꼼꼼히 보시고 자유롭게 코멘트 남겨주시면 감사하겠습니다.

Core Code

. . .
	@Test
	void 카테고리로_케이크_이미지_조회에_성공한다() {
		// given
		final String url = "%s%d%s/search/categories".formatted(BASE_URL, port, API_URL);
		final UriComponents uriComponents = UriComponentsBuilder
			.fromUriString(url)
			.queryParam("category", CakeDesignCategory.FLOWER)
			.queryParam("pageSize", 5)
			.build();

		// when
		final ResponseEntity<ApiResponse> responseEntity = restTemplate.getForEntity(uriComponents.toUriString(), ApiResponse.class);

		// then
		final ApiResponse response = objectMapper.convertValue(responseEntity.getBody(), ApiResponse.class);
		final CakeImageListResponse data = objectMapper.convertValue(response.getData(), CakeImageListResponse.class);

		Assertions.assertEquals(HttpStatusCode.valueOf(200), responseEntity.getStatusCode());
		Assertions.assertEquals(ReturnCode.SUCCESS.getCode(), response.getReturnCode());
		Assertions.assertEquals(ReturnCode.SUCCESS.getMessage(), response.getReturnMessage());
		Assertions.assertEquals(5, data.cakeImages().size());
		Assertions.assertEquals(6L, data.lastCakeId());
		Assertions.assertEquals(5, data.size());
	}
. . .

다음은 커서 페이지네이션 쿼리입니다.

select  cs1_0.shop_id,  c1_0.cake_id,  c1_0.cake_image_url 
from  cake c1_0 
join  cake_shop cs1_0  on c1_0.shop_id=cs1_0.shop_id 
join  cake_category cc1_0  on cc1_0.cake_id=c1_0.cake_id 
where  c1_0.cake_id<?  and cc1_0.cake_design_category=? 
order by  c1_0.cake_id desc 
limit  ?

etc

@lcomment lcomment added chore 빌드 매니저 및 환경 설정 feature 새로운 기능 개발 labels May 22, 2024
@lcomment lcomment requested a review from YongsHub May 22, 2024 09:56
@lcomment lcomment self-assigned this May 22, 2024
Copy link

github-actions bot commented May 22, 2024

Test Results

16 tests  +3   15 ✅ +3   2s ⏱️ -1s
 5 suites +1    1 💤 ±0 
 5 files   +1    0 ❌ ±0 

Results for commit 540d6d7. ± Comparison against base commit ca96053.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@YongsHub YongsHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다

Assertions.assertEquals(ReturnCode.SUCCESS.getCode(), response.getReturnCode());
Assertions.assertEquals(ReturnCode.SUCCESS.getMessage(), response.getReturnMessage());
Assertions.assertEquals(5, data.cakeImages().size());
Assertions.assertEquals(6L, data.lastCakeId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lastCakeId가 6L에 대해 검증하는건 테스트 코드 조건에 따라 변경될 수 있는 부분 같은데
CakeDesignCategory.FLOWER에 대한 검증 유무가 더 적합한 테스트라는 생각이 드네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

카테고리에 대한 검증도 추가하겠습니다. 6L에 대한건 페이지네이션 검증이었습니다만, 정말 last id가 6L인지 검증을 추가하겠습니다.

Assertions.assertEquals(ReturnCode.SUCCESS.getCode(), response.getReturnCode());
Assertions.assertEquals(ReturnCode.SUCCESS.getMessage(), response.getReturnMessage());
Assertions.assertEquals(5, data.cakeImages().size());
Assertions.assertEquals(1L, data.lastCakeId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 Id 값에 대해 1L라는걸 검증하려면 테스트 queryParam에서 cakeId가 6이니까 제목에서 내림차순 쿼리 정렬이 진행되는게 표현되면 좋을 것 같습니다
혹은 실제 판단하는 목적이 카테고리에 초점이 맞춰지면 더 좋은 테스트이지 아닐까 싶어요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네, 확인했습니다. 제목 표기는 어떻게 생각하시나요? 태용님이 메서드명에 하시길래 일단 컨벤션을 맞춰보았는데, 아무래도 언더바만 사용한다는 점 등 표기에 제한이 있더라구요. 또 코드상으로 보기엔 너무 가독성이 떨어져서요. (테스트 코드는 문서라는 말이 많은데, 가독성이 떨어지면 문서 역할을 잘 못한다고 생각합니다) 현재 방식 유지가 좋을까요? 아니면 @DisplayName 어노테이션을 활용하는게 좋을까요?? 혹시 메서드 네임으로 하시면서 불편하시진 않았나요??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DisplayName을 활용해볼까요??
메서드 네임으로 하면 저도 한계점이 보이는거 같아요

Comment on lines +20 to +26
@SqlGroup({
@Sql(scripts = {
"/sql/insert-test-user.sql",
"/sql/insert-cake.sql"
}, executionPhase = BEFORE_TEST_METHOD),
@Sql(scripts = "/sql/delete-all.sql", executionPhase = AFTER_TEST_METHOD)
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 애노테이션이 추상 클래스에서 선언되는건 안되던가요?? 기억이 잘 안나서 헷갈리네용

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스크립트 파일이 테스트마다 다를 것이기에 추상화를 못했습니다

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이해했습니다!

@lcomment lcomment merged commit 5afd991 into master May 23, 2024
2 checks passed
@lcomment lcomment deleted the feature/#22 branch May 23, 2024 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 빌드 매니저 및 환경 설정 feature 새로운 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants